Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add exdoc:loaded event #2069

Merged
merged 7 commits into from
Jan 25, 2025
Merged

Add exdoc:loaded event #2069

merged 7 commits into from
Jan 25, 2025

Conversation

josevalim
Copy link
Member

Closes #2068.

Copy link

github-actions bot commented Jan 24, 2025

@jonatanklosko
Copy link
Member

Just one commend and looks good to me. @liamcmitchell please let us know if you have any concerns, thanks!

It may be worth documenting exdoc:loaded explicitly in the readme section, and say that: (a) it may run multiple times, so initialization that should happen once must be conditional; (b) for external scripts they should use defer, but not async.

Co-authored-by: Jonatan Kłosko <[email protected]>
@josevalim
Copy link
Member Author

@garazdawi you will have to update Erlang. I plan to release v0.37.0-rc.0 this weekend, which should also be compatible with Erlang/OTP 28.

@@ -89,7 +89,7 @@ export function initialize () {
})

window.addEventListener('hashchange', markCurrentHashInSidebar)
window.addEventListener('swup:page:view', markCurrentHashInSidebar)
window.addEventListener('exdoc:loaded', markCurrentHashInSidebar)

markCurrentHashInSidebar()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now duplicated and could be removed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I will push it soon. And to be clear, the next line does:

  // Triggers layout, defer.
  requestAnimationFrame(scrollNodeListToCurrentCategory)

That would still be fine, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I can confirm this one breaks it and nothing is being marked on the sidebar. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will ship a new version with this still using swup:page:view. If you have suggestions on how to unify it, it would be very welcome. :) Thank you ❤️

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I missed that this init is lazy and will always come after the first exdoc:loaded. Pushed changes to unify and make more clear in #2070

@josevalim josevalim merged commit bb89096 into main Jan 25, 2025
8 checks passed
@josevalim
Copy link
Member Author

💚 💙 💜 💛 ❤️

@josevalim josevalim deleted the jv-exdoc-loaded branch January 25, 2025 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Rendering Mermaid graph after navigation
3 participants